Skip to content

ci(worker): gate the integration tests (non-short job + DB services in ci.yml + deploy.yml) - #88

Merged
mastermanas805 merged 1 commit into
masterfrom
ci/worker-integration-db-job
Jun 4, 2026
Merged

ci(worker): gate the integration tests (non-short job + DB services in ci.yml + deploy.yml)#88
mastermanas805 merged 1 commit into
masterfrom
ci/worker-integration-db-job

Conversation

@mastermanas805

@mastermanas805 mastermanas805 commented Jun 4, 2026

Copy link
Copy Markdown
Member

Problem: the worker integration tests skipped EVERYWHERE and gated nothing

The worker's real-DB integration tests (internal/jobs/*_integration_test.go,
and the Wave-1 internal/testhelpers harness) t.Skip under -short and
when TEST_DATABASE_URL is unset. But every worker test workflow ran them in
a mode where they skip:

  • ci.yml build-and-test: go test ./... -v -race -count=1, no DB
    service, no TEST_DATABASE_URL     → skip (no DB).
  • deploy.yml "Run unit tests": go test ./... -short → skip (-short).
  • coverage.yml: has pg+redis+mongo services but runs -short → skip.

Net: the trigger→DB-effect round-trip these tests exist to assert ran on
exactly one developer's laptop and gated nothing in CI. The fix is NOT
"add a Postgres service" (coverage.yml already has one) — it's running the
integration tests without -short in a gating workflow.

Fix

Add a dedicated integration job that runs the real-DB integration tests
without -short against a real Postgres + Redis, reusing coverage.yml's
proven service block + api-migrations-apply step. Scoped via -run Integration
(all integration test funcs carry "Integration" in their name) at -p 1 (the
tests share one DB). No build-tag retrofit (Mechanism C, per the decision doc).

Two-gate landing (root CLAUDE.md rule 15 / project_api_two_test_gates_ci_and_deploy)

A worker test-infra change must land in both ci.yml and deploy.yml, or
the deploy wedges while PR CI stays green — the exact failure hit 2026-05-23
with the NATS service. So:

  • ci.yml — new integration job (PR + push gate).
  • deploy.yml — mirror integration job; deploy now needs: integration
    so a red integration round-trip blocks build/rollout.

The fast -short jobs (build-and-test in ci.yml, "Run unit tests" in
deploy.yml) are unchanged — make gate parity (rule 23) holds.

Pre-existing test bug surfaced by the non-short run

TestPropagation_ForUpdateSkipLockedIntegration seeded a pending_propagations
row referencing a non-existent team, which violates
pending_propagations_team_id_fkey → teams(id) against a fully api-migrated DB
(i.e. the new job's environment). Without fixing it, the new job would red on a
pre-existing bug. The test now seeds the parent team first; the FK ON DELETE CASCADE tidies the propagation row on cleanup.

Verification (local, against postgres://postgres@localhost:5432/instant_dev_test)

  • go test ./internal/... -run Integration -p 1 (no -short) → all pass, exit 0;
    the live-DB tests RUN (no longer skip).
  • Proof the gate bites: a temporary broken assert in the live-DB SKIP-LOCKED
    test (if total > 1 → if total >= 0) → the non-short run FAILS; reverted, green.
  • make gate (the -short path) still green; integration tests skip there as before.
  • Both workflow files validated with python3 -c 'import yaml; yaml.safe_load(...)'.

🤖 Generated with Claude Code

@mastermanas805 @claude
ci(worker): gate the integration tests (non-short job + DB services)
6f1d7f5 
The worker's real-DB integration tests (internal/jobs/*_integration_test.go)
skip under `-short` AND when TEST_DATABASE_URL is unset (see
propagation_runner_integration_test.go and the testhelpers harness guard).
But EVERY worker test workflow ran with `-short` (deploy.yml) or `-race`
with no DB service (ci.yml's build-and-test) — and coverage.yml runs
`-short` too. Net: the integration tests skipped EVERYWHERE in CI and
gated NOTHING. They only ran on a developer box with a DB up.

Fix: add a dedicated GATING `integration` job that runs the real-DB
integration tests WITHOUT `-short` against a real Postgres+Redis,
reusing coverage.yml's proven service block + api-migrations-apply step.

Per the two-gate rule (root CLAUDE.md rule 15 /
project_api_two_test_gates_ci_and_deploy — hit 2026-05-23 with the NATS
service), a worker test-infra change must land in BOTH ci.yml and
deploy.yml or the deploy wedges while PR CI stays green. So:
- ci.yml: new `integration` job (PR + push gate).
- deploy.yml: mirror `integration` job; `deploy` now `needs: integration`
  so a red integration round-trip blocks build/rollout.

The fast `-short` `build-and-test` (ci.yml) and "Run unit tests"
(deploy.yml) jobs are unchanged — `make gate` parity (rule 23) holds.

Also fixes a pre-existing FK violation in
TestPropagation_ForUpdateSkipLockedIntegration: it seeded a
pending_propagations row referencing a non-existent team, which fails
against a fully api-migrated DB (the FK pending_propagations_team_id_fkey
→ teams). Without this the new non-short job would red on a pre-existing
bug. The test now seeds the parent team first; the FK cascade tidies it.

Verified locally against postgres://postgres@localhost:5432/instant_dev_test:
- non-short `go test ./internal/... -run Integration -p 1` → all pass (exit 0).
- a temporary broken assert in the live-DB SKIP-LOCKED test → reds the run.
- `make gate` (the `-short` path) still green; integration tests skip there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mastermanas805
mastermanas805 enabled auto-merge (squash) June 4, 2026 16:16
@mastermanas805
mastermanas805 merged commit 06e12b0 into master Jun 4, 2026
12 checks passed
@mastermanas805 mastermanas805 mentioned this pull request Jun 4, 2026
Merged
6 tasks
mastermanas805 added a commit that referenced this pull request Jun 4, 2026
@mastermanas805 @claude
test(worker): Tier-1 job integration round-trips (billing/deletion)
eccd0d6 
Add real-Postgres integration round-trips for the two highest-value
Tier-1 (data-loss / money-adjacent) worker jobs flagged in
INTEGRATION-COVERAGE-PLAN-2026-06-04.md §5 #1, building on the Wave-1
testhelpers harness (#87) and the non-short CI integration gate (#88).

billing_reconciler — terminal downgrade:
  - seed a real pro team with a Razorpay subscription id, run Work() with
    a sub-keyed stub fetcher reporting `cancelled`, assert the LIVE row
    flipped plan_tier pro→hobby (terminalDowngradeTier, never 'free' —
    D28 F1) AND a subscription.canceled audit_log row was emitted.
  - complement: an `active`-at-current-tier team is left untouched (no
    spurious UPDATE / upgrade email every tick).

team_deletion_executor — purge cascade (the §5 #1 single biggest gap):
  - seed a team past its 30-day grace window + secret-bearing resources +
    PII-bearing users + a deployment, run Work() with provisioner/S3/k8s
    nil (fail-open), assert the persisted cascade: status→tombstoned,
    resources.connection_url→NULL / key_prefix→'', users.email scrubbed
    to the deleted-<id>@tombstoned.invalid placeholder, team.tombstoned
    audit emitted.
  - complement: an in-grace team (5 days) is NOT swept — secrets intact,
    no tombstone (truehomie-class "destroy ran when it shouldn't" guard).

Gate-filter finding: all functions carry "Integration" so the #88 gate
(`go test ./internal/... -run Integration`) executes them; the Wave-1
#87 tests already match (TestIntegration_* / TestPropagation_*Integration*)
so NO rename was needed. All seeds use testhelpers.SetupTestDB's -short/
no-DB skip guard, so `make gate` stays green and they run only under the
non-short integration job.

Harness additions (internal/testhelpers/billing_deletion.go): team
sub/tier/status + audit-by-team + pending-deletion + secret-resource +
user seeds/readbacks. SeedTeamWithSubscription seeds a low-sorting team id
so the reconciler's ORDER BY id LIMIT 100 candidate scan always reaches it
regardless of DB pollution (deterministic on fresh CI and busy local DB).

Integration-cov delta (jobs pkg, -run Integration, -coverpkg jobs):
5.6% → 8.3% (+2.7pp). Touched: billing_reconciler.Work 45.3%,
updatePlanTier 75%, emitCancelAudit 66.7%; team_deletion_executor.Work
76.2%, processTeam 40.8%, emitTombstoned 80%, fetchCandidates 83.3%
(all 0% from the integration suite before).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805 added a commit that referenced this pull request Jun 4, 2026
@mastermanas805 @claude
test(worker): Tier-1 job integration round-trips (billing/deletion) —…
baa1aba 
… toward worker coverage floor (#89)

* test(worker): Tier-1 job integration round-trips (billing/deletion)

Add real-Postgres integration round-trips for the two highest-value
Tier-1 (data-loss / money-adjacent) worker jobs flagged in
INTEGRATION-COVERAGE-PLAN-2026-06-04.md §5 #1, building on the Wave-1
testhelpers harness (#87) and the non-short CI integration gate (#88).

billing_reconciler — terminal downgrade:
  - seed a real pro team with a Razorpay subscription id, run Work() with
    a sub-keyed stub fetcher reporting `cancelled`, assert the LIVE row
    flipped plan_tier pro→hobby (terminalDowngradeTier, never 'free' —
    D28 F1) AND a subscription.canceled audit_log row was emitted.
  - complement: an `active`-at-current-tier team is left untouched (no
    spurious UPDATE / upgrade email every tick).

team_deletion_executor — purge cascade (the §5 #1 single biggest gap):
  - seed a team past its 30-day grace window + secret-bearing resources +
    PII-bearing users + a deployment, run Work() with provisioner/S3/k8s
    nil (fail-open), assert the persisted cascade: status→tombstoned,
    resources.connection_url→NULL / key_prefix→'', users.email scrubbed
    to the deleted-<id>@tombstoned.invalid placeholder, team.tombstoned
    audit emitted.
  - complement: an in-grace team (5 days) is NOT swept — secrets intact,
    no tombstone (truehomie-class "destroy ran when it shouldn't" guard).

Gate-filter finding: all functions carry "Integration" so the #88 gate
(`go test ./internal/... -run Integration`) executes them; the Wave-1
#87 tests already match (TestIntegration_* / TestPropagation_*Integration*)
so NO rename was needed. All seeds use testhelpers.SetupTestDB's -short/
no-DB skip guard, so `make gate` stays green and they run only under the
non-short integration job.

Harness additions (internal/testhelpers/billing_deletion.go): team
sub/tier/status + audit-by-team + pending-deletion + secret-resource +
user seeds/readbacks. SeedTeamWithSubscription seeds a low-sorting team id
so the reconciler's ORDER BY id LIMIT 100 candidate scan always reaches it
regardless of DB pollution (deterministic on fresh CI and busy local DB).

Integration-cov delta (jobs pkg, -run Integration, -coverpkg jobs):
5.6% → 8.3% (+2.7pp). Touched: billing_reconciler.Work 45.3%,
updatePlanTier 75%, emitCancelAudit 66.7%; team_deletion_executor.Work
76.2%, processTeam 40.8%, emitTombstoned 80%, fetchCandidates 83.3%
(all 0% from the integration suite before).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(testhelpers): in-package coverage for billing_deletion.go harness

billing_deletion.go is exercised only by internal/jobs integration tests, so
Go credits its line coverage to internal/jobs, not internal/testhelpers — the
diff-cover --fail-under=100 patch gate read it as 0% (142 missing lines),
reddening PR #89's coverage check. Same root cause/fix as the merged #87
testhelpers.go fix.

Add billing_deletion_smoke_test.go giving the harness its own in-package
coverage: a DB-backed happy-path round-trip (gated on DB reachability via
SetupTestDB, runs in coverage.yml which exports TEST_DATABASE_URL + applies
api migrations), a closed-DB error-arms test, lowSortingTeamID unit coverage,
and a withoutGithubIDColumn seam exercising SeedUser's fallback INSERT branch.

Route each helper's failure arm through the package tFatalf seam (matching
testhelpers.go) so the error arms are coverable via seam-swap — a test seam,
not a behavioural change; real callers still get genuine t.Fatalf.

billing_deletion.go is now 100.0% covered in-package (go tool cover -func).
No threshold lowered, no waiver added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mastermanas805